Kestrel-3

Verilog Development Checklist
Login

VERILOG CHECKLIST

Coding in Verilog? You're risking going crazy unless you follow these checklist items.

Compilation Units

  1. `default_nettype none. This is unconditional. Patches will be refused if your Verilog modules lack this declaration. Not including this has given me (no joke!) months of grief when debugging.

  2. Prefer parameters over preprocessor directives. (This is placed at the module level since preprocessor definitions are global in scope.) Are they really necessary? Prefer parameters and generate blocks over preprocessor directives when and where possible. They take less typing to use on-average, which means fewer mistakes.

module Headers

  1. All signals are listed without qualification. For example, it's not input foo or output [63:0] bar, it's just foo or bar.

  2. There is a comma separating every signal name.

  3. There is not a comma after the last signal name.

  4. If you do not employ the Haskell-style module declaration (see below), list i_clk and i_reset as the last signals in the module. If you do employ the Haskell-style module declaration method, list i_clk and i_reset as the first signals in the module. You'll never have to change these signals, so if you need to rearrange signal groupings later on, you can do so without concern for the separating commas.

  5. All inputs should start with i_.

  6. All outputs should start with o_.

  7. All bidirectional signals should be split into a distinct input and output, along with an output enable output to indicate data direction to external logic or I/O blocks.

  8. Groups of related signals (as with a bus or single port) should have its mnemonic name follow the i_ or o_ prefix. For example, i_master_valid or o_slave_ready. Never use master_i_valid, or the like. Deprecate formats like master_valid_i.

  9. All signal names should be spelled with lowercase characters. Uppercase names are deprecated.

  10. All parameters should be in all upper-case notation, just as preprocessor symbols would be. They serve a similar purpose as preprocessor macros, so their nomenclature should resemble this.

  11. Parameters that cannot be changed by the instantiating module, but are good practice to name anyway, should be localparams

NOTE: A lot of my own Verilog sources will break at least one of these rules. That's because I've not suffered pain from violating these rules at the time I wrote them. They will be modernized over time on an as-needed basis.

BAD

module FIFO(
    input i_reset,
    input i_clk,

    input i_a_valid,
    output i_a_ready,
    input [`UPB:0] i_a_byte,

    output o_b_valid,
    input i_b_ready,
    output [`UPB:0] o_b_byte
);

GOOD

module FIFO(
    i_a_valid,
    i_a_ready,
    i_a_byte,

    o_b_valid,
    i_b_ready,
    o_b_byte,

    i_reset,
    i_clk
);
parameter BUS_WIDTH = 8;
localparam UPB = (BUS_WIDTH-1);

input wire [UPB:0] i_a_byte;
output wire [UPB:0] o_b_byte;
reg [UPB:0] r_b_byte;
assign o_b_byte = r_b_byte;

ALSO GOOD

I call this the Haskell-influenced module header. Haskell programmers will be able to readily identify why.

module FIFO(
    i_reset
,   i_clk

,   i_a_valid
,   i_a_ready
,   i_a_byte

,   o_b_valid
,   i_b_ready
,   o_b_byte
);
parameter BUS_WIDTH = 8;
localparam UPB = (BUS_WIDTH-1);

input wire [UPB:0] i_a_byte;
output wire [UPB:0] o_b_byte;
reg [UPB:0] r_b_byte;
assign o_b_byte = r_b_byte;

WHY

Bluntly, it's because literally nobody can agree on a common subset of available Verilog syntaxes when it comes to module declarations, and I'm frankly tired of dealing with it. As they say in the Python community, explicit is better than implicit.

Of course, at some point, you'll need to qualify the module signals. That leads us to . . .

Signals and Assignments

Some of these are merely stylistic issues. I don't necessarily agree with all of them, but in the interest of supporting a burgeoning open-hardware community, I'd rather a consistent coding convention be used to facilitate easier code sharing.

  1. Always use 0 for a lower bound on a bus. If you really want to use a non-zero lower-bound, make sure assignments are fully qualified. ZipCPU puts it best, I think: I recently got burned with a "wire [22:5] value;" declaration. Setting value to 1 gave me some problems, as Yosys tried to synthesize a bit [0]. I normally always use a 0 LSB, but in this case the [22:5] made a lot of things easier to read. Hence, I'd suggest preferring a zero LSB or (if not) always specifying which bits you are talking about. value[22:5]=1 worked, value = 1 did not.

  2. For blocking assignments, always use always @(*). Again, from ZipCPU's storied experience: I use blocking assignments only in always @() blocks--keeps Verilator from complaining about them.*

  3. Avoid assigns when using ?: operator. ZipCPU writes: If I have more than one ?: operator, I try to avoid assign's, choosing instead to use an always @()* (todo -- find out why this is.)

  4. Avoid initial blocks. They are a great aid for simulation, but have variable semantics across vendors of (and, sometimes, families offered by a single vendor of) FPGAs and aren't supported at all by ASIC workflows. Replace with explicit reset logic if their value is important. Note that these are different from initializers. Please see below for more details about initializers.

Output Signals

  1. All outputs are wires. Unconditionally. Forever and ever. Do not trust output reg.

  2. If an output must be a register, instantiate it manually.

  3. NEVER use an initializer with an output statement.

  4. Qualify and declare your outputs next to the process that is responsible for driving them.

NOTE: It's technically valid Verilog to write an initializer as long as it is a constant expression. However, for the sake of uniformity and reducing the amount of special-cases the developer has to be concerned with, *just don't.

If an output X is to be driven by a register, then you should have the following declarations in the source file:

reg r_X;
output wire o_X;
assign o_X = r_X;

If X is a bus:

reg [UB:LB] r_X;
output wire [UB:LB] o_X;
assign o_X[UB:LB] = r_X;

If a signal is hard-wired to a specific value:

output wire [2:0] o_foo;
assign o_foo = 3'b101;

BAD

module foo(
    // ...
    output o_bar
);

    // 1500 lines of gibberish

    always @(posedge i_clk) begin
        // 150 more lines of irrelevant code
        if(some_condition) begin
            o_bar <= ...etc...;
        end
    end

endmodule;

GOOD

module foo(
    // ...
    o_bar,
    // ...
)

    // 1500 lines of gibberish

    output wire o_bar;
    reg r_bar;
    assign o_bar = r_bar;

    always @(posedge i_clk) begin
        if(i_reset) r_bar <= `BAR_RESET_VALUE;
        else if(some_condition) r_bar <= `SOME_OTHER_VALUE;
        else ...
    end

    // etc...
endmodule

WHY

This is damn annoying to have to do, but it's been a major source of heart-ache when trying to unify the results of formal verification against testing tools like Verilator. It seems nobody can detect illegal variations of the output X = Y; statement (where Y is not a constant). Therefore, avoid them all-together. Don't even look down that dark alley.

(I started to get into this bad habit because of Xilinx tools which not only accept but also encourages this style of coding.)

Moreover, colocating the registers, assignments, and state transition logic has made things easier to:

  1. Document. I find that my documentation about the effects of a piece of state tends to cluster around a single spot in the source code, instead of being scattered about the source.

  2. Easier to widen/narrow. Signals can often become buses, buses can often widen, or buses can often become narrower depending on your development trajectory.

  3. Easier to just delete everything. It's quite satisfying when removing dead code that is no longer relevant.

Input Signals

  1. If there's no better place to declare a signal as an input, declare it immediately after the module statement.

  2. If an input is going to be consumed by only one chunk of code, it's best to declare it immediately ahead of that code.